split TypeChecker.fs #10317
split TypeChecker.fs #10317
Conversation
|
but what will we use as benchmark after this? :P |
LOL I was just adjusting those benchmarks and thinking "oh wow, this will speed them up" |
|
BTW it feels like a good time to do this split, as AFAIK there are relatively few outstanding changes to TypeChecker.fs in proposed features or PRs |
|
Hmmm transient failure on Mac |
|
@cartermp I'm loving the fact that the compiler guide is in the tree and we can search and replace to help update it |
KevinRansom
left a comment
KevinRansom
left a comment
There was a problem hiding this comment.
Thanks Don, this is good.
cartermp
left a comment
cartermp
left a comment
There was a problem hiding this comment.
My approval is one of happiness and sadness.
Happiness that this is finally getting split up into some logical chunks!
Sadness that, like @forki, I no longer have this ridiculous amazing benchmark for various tooling experience smoke tests and benchmarks.
|
I'm just amazed that now GitHub will actually render the content of the files, so we can deep-link directly to relevant code when showing folks around. |
|
Yes it's the end of an era. But at least there are still 230 character lines in the files to enjoy. |
| errorR(Error(FSComp.SR.tcInvalidNamespaceModuleTypeUnionName(), id.idRange)) | ||
|
|
||
| let CheckDuplicates (idf: _ -> Ident) k elems = | ||
| elems |> List.iteri (fun i uc1 -> |
There was a problem hiding this comment.
I suggest a small optimization here:
let CheckDuplicates (idf: _ -> Ident) k elems =
let ids = elems |> List.mapi (fun i uc -> i, idf uc)
ids
|> List.iter (fun (i, id1) ->
ids
|> List.iter (fun (j, id2) ->
if j > i && id1.idText = id2.idText then
errorR (Duplicate(k, id1.idText, id1.idRange))))
elems
because is computing idf for all elements too many times n * (n + 1) instead of just n
There was a problem hiding this comment.
could you please send it as new PR. I'd also suggest you unroll it as nested for loops .
There was a problem hiding this comment.
The suggested code will look like this:
let CheckDuplicates (idf: _ -> Ident) k elems =
let ids = elems |> List.mapi (fun i uc -> i, idf uc)
for (i, id1) in ids do
for (j, id2) in ids do
if j > i && id1.idText = id2.idText then
errorR (Duplicate(k, id1.idText, id1.idRange))
elems
7 participants
This is cleanup to split TypeChecker.fs into two chunks of 12,000 and 6,000 lines.
The chunks are
CheckExprs.fs - check syntactic types and expressions, and many aspects of members and recursion (since object expressions include members)
CheckDecls.fs - check type declarations, modules, namespaces, signatures, files
This necessarily involves opening up some of the internals of TypeChecker.fs in the signature of CheckExprs.fsi but I think that's ok. More is opened up than I would like (and we may be able to reduce that in the future) but it's not too bad.
We can do further splitting at a later point, e.g. split out
CheckComputationExprs.fs,CheckObjectExprs.fs,TcFileState.fs,TcEnv.fsand so on (if that proves the right split)